Skip to content

fix(supervisor): drop sandbox child capability bounding set#2001

Merged
alangou merged 1 commit into
mainfrom
alangou/os-173-high-docker-sandboxes-run-as-root-with-sys_admin-and
Jun 29, 2026
Merged

fix(supervisor): drop sandbox child capability bounding set#2001
alangou merged 1 commit into
mainfrom
alangou/os-173-high-docker-sandboxes-run-as-root-with-sys_admin-and

Conversation

@alangou

@alangou alangou commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Drop the Linux capability bounding set in the common sandbox child privilege-drop path so workloads and openshell connect shells cannot regain container-granted capabilities after exec.

Related Issue

Closes #1452

Changes

  • Added Linux-only capability bounding set reduction in drop_privileges() after initgroups/setgid and before setuid.
  • Drops capabilities from 0..=cap_last_cap, with CAP_SETPCAP dropped last and a graceful fallback when CAP_SETPCAP is unavailable.
  • Added focused Linux-only regression coverage and documented the child-process invariant in architecture/sandbox.md.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E not applicable; no e2e files changed

Additional checks run:

  • mise run fmt && mise run e2e && mise run e2e:kubernetes && mise run e2e:docker
  • cargo test -p openshell-supervisor-process --lib process::tests -- --nocapture
  • cargo test -p openshell-supervisor-process --lib ssh::tests::pre_exec_always_calls_drop_privileges -- --nocapture

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)

Comment thread crates/openshell-supervisor-process/src/process.rs Outdated

@elezar elezar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A codex-assisted review:

I think this still fails closed incorrectly for the Podman path. drop_capability_bounding_set() returns Ok(()) when CAP_SETPCAP is
not effective, and also when the first PR_CAPBSET_DROP returns EPERM. But the Podman driver currently drops SETPCAP from the
supervisor container, so that path leaves the child bounding set unchanged while still spawning the workload/connect shell.

Could we either keep SETPCAP available to the supervisor until child setup, or fail the spawn when the bounding set cannot be cleared
and is still nonempty? This may also be a good place to use capctl rather than custom /proc parsing and raw prctl;
capctl::caps::bounding::clear() plus an explicit “EPERM is only OK if the bounding set is already empty” check would make the invariant
clearer.

The current regression test skips when CAP_SETPCAP is unavailable, so it would not catch the Podman-relevant failure mode.

@johntmyers

Copy link
Copy Markdown
Collaborator

A codex-assisted review:

I think this still fails closed incorrectly for the Podman path....

+1. Seems like different drivers can do different drops on their own which would impact this common code that runs driver-agnostically.

@alangou alangou force-pushed the alangou/os-173-high-docker-sandboxes-run-as-root-with-sys_admin-and branch 3 times, most recently from 0c0c87e to b629752 Compare June 26, 2026 13:11
@alangou alangou requested a review from elezar June 26, 2026 13:27
@alangou

alangou commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

@johntmyers @elezar I have implemented the requested changes

Comment thread crates/openshell-server/src/compute/mod.rs Outdated
@johntmyers

Copy link
Copy Markdown
Collaborator

looks good @alangou - will need to update the README for openshell-driver-podman as it still mentions SETPCAP is dropped

Reduce the Linux capability bounding set in the common privilege-drop path before executing sandbox workloads or connect shells and use capctl

Signed-off-by: Adrien Langou <alangou@nvidia.com>
@alangou alangou force-pushed the alangou/os-173-high-docker-sandboxes-run-as-root-with-sys_admin-and branch from 6acb0bc to b0bb43b Compare June 29, 2026 08:44
@alangou

alangou commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

@johntmyers the readme is updated (auto-merge is enable)

@alangou alangou enabled auto-merge (squash) June 29, 2026 09:30
@elezar elezar added the test:e2e Requires end-to-end coverage label Jun 29, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied for b0bb43b. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@alangou alangou merged commit afc06dd into main Jun 29, 2026
62 checks passed
@alangou alangou deleted the alangou/os-173-high-docker-sandboxes-run-as-root-with-sys_admin-and branch June 29, 2026 15:10
ben-alkov pushed a commit to ben-alkov/fullsend that referenced this pull request Jun 30, 2026
The OpenShell gateway defaults to pulling
ghcr.io/nvidia/openshell/supervisor:latest for the sandbox supervisor
binary. When NVIDIA released v0.0.73 (2026-06-30 15:31 UTC), the
:latest tag was re-pointed to a supervisor that drops the Linux
capability bounding set (NVIDIA/OpenShell#2001), which crashes with
EINVAL in rootless Podman on GitHub Actions runners.

Write a gateway.toml that pins supervisor_image to the version from
openshell-version.sh so the supervisor always matches the installed
gateway and is immune to upstream :latest tag changes.

Fixes fullsend-ai#2792

Assisted-by: Claude (investigation, fix)
Signed-off-by: Wayne Sun <gsun@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(sandbox): support --cap-drop / capability scoping on sandbox create

3 participants